From 1ccdc8bb1d52eb2139163ee33dca7f5cd86d46cf Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Fri, 9 Jun 2017 13:07:06 +0300 Subject: [PATCH] Update a credentials file format. Implement tests for login command. --- src/cargo/util/config.rs | 105 ++++++++++++++++++++++++------------ tests/login.rs | 112 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 35 deletions(-) create mode 100644 tests/login.rs diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index b99285ce7..fe828c107 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -5,6 +5,7 @@ use std::collections::HashSet; use std::env; use std::fmt; use std::fs::{self, File}; +use std::io::SeekFrom; use std::io::prelude::*; use std::mem; use std::path::{Path, PathBuf}; @@ -429,28 +430,50 @@ impl Config { Ok(()) }).chain_err(|| "Couldn't load Cargo configuration")?; - let mut map = match cfg { - CV::Table(map, _) => map, + self.load_credentials(&mut cfg)?; + match cfg { + CV::Table(map, _) => Ok(map), _ => unreachable!(), - }; + } + } + fn load_credentials(&self, cfg: &mut ConfigValue) -> CargoResult<()> { let home_path = self.home_path.clone().into_path_unlocked(); - let token = load_credentials(&home_path)?; - if let Some(t) = token { - if !t.is_empty() { - let mut registry = map.entry("registry".into()) - .or_insert(CV::Table(HashMap::new(), PathBuf::from("."))); - match *registry { - CV::Table(ref mut m, _) => { - m.insert("token".into(), - CV::String(t, home_path.join("credentials"))); - } - _ => unreachable!(), - } - } + let credentials = home_path.join("credentials"); + if !fs::metadata(&credentials).is_ok() { + return Ok(()); } - Ok(map) + let mut contents = String::new(); + let mut file = File::open(&credentials)?; + file.read_to_string(&mut contents).chain_err(|| { + format!("failed to read configuration file `{}`", credentials.display()) + })?; + + let toml = cargo_toml::parse(&contents, + &credentials, + self).chain_err(|| { + format!("could not parse TOML configuration in `{}`", credentials.display()) + })?; + + let value = CV::from_toml(&credentials, toml).chain_err(|| { + format!("failed to load TOML configuration from `{}`", credentials.display()) + })?; + + let mut cfg = match *cfg { + CV::Table(ref mut map, _) => map, + _ => unreachable!(), + }; + + let mut registry = cfg.entry("registry".into()) + .or_insert(CV::Table(HashMap::new(), + PathBuf::from("."))); + registry.merge(value).chain_err(|| { + format!("failed to merge configuration at `{}`", + credentials.display()) + })?; + + Ok(()) } /// Look for a path for `tool` in an environment variable or config path, but return `None` @@ -567,6 +590,21 @@ impl ConfigValue { } } + fn into_toml(self) -> toml::Value { + match self { + CV::Boolean(s, _) => toml::Value::Boolean(s), + CV::String(s, _) => toml::Value::String(s), + CV::Integer(i, _) => toml::Value::Integer(i), + CV::List(l, _) => toml::Value::Array(l + .into_iter() + .map(|(s, _)| toml::Value::String(s)) + .collect()), + CV::Table(l, _) => toml::Value::Table(l.into_iter() + .map(|(k, v)| (k, v.into_toml())) + .collect()), + } + } + fn merge(&mut self, from: ConfigValue) -> CargoResult<()> { match (self, from) { (&mut CV::String(..), CV::String(..)) | @@ -774,8 +812,21 @@ pub fn save_credentials(cfg: &Config, "credentials' config file")? }; - file.write_all(token.as_bytes())?; - file.file().set_len(token.len() as u64)?; + let mut contents = String::new(); + file.read_to_string(&mut contents).chain_err(|| { + format!("failed to read configuration file `{}`", + file.path().display()) + })?; + let mut toml = cargo_toml::parse(&contents, file.path(), cfg)?; + toml.as_table_mut() + .unwrap() + .insert("token".to_string(), + ConfigValue::String(token, file.path().to_path_buf()).into_toml()); + + let contents = toml.to_string(); + file.seek(SeekFrom::Start(0))?; + file.write_all(contents.as_bytes())?; + file.file().set_len(contents.len() as u64)?; set_permissions(file.file(), 0o600)?; return Ok(()); @@ -796,19 +847,3 @@ pub fn save_credentials(cfg: &Config, Ok(()) } } - -fn load_credentials(home: &PathBuf) -> CargoResult> { - let credentials = home.join("credentials"); - if !fs::metadata(&credentials).is_ok() { - return Ok(None); - } - - let mut token = String::new(); - let mut file = File::open(&credentials)?; - file.read_to_string(&mut token).chain_err(|| { - format!("failed to read configuration file `{}`", - credentials.display()) - })?; - - Ok(Some(token.trim().into())) -} diff --git a/tests/login.rs b/tests/login.rs new file mode 100644 index 000000000..306b4e0d9 --- /dev/null +++ b/tests/login.rs @@ -0,0 +1,112 @@ +#[macro_use] +extern crate cargotest; +extern crate cargo; +extern crate hamcrest; +extern crate toml; + +use std::io::prelude::*; +use std::fs::{self, File}; + +use cargotest::cargo_process; +use cargotest::support::execs; +use cargotest::support::registry::registry; +use cargotest::install::cargo_home; +use hamcrest::{assert_that, existing_file, is_not}; + +const TOKEN: &str = "test-token"; +const CONFIG_FILE: &str = r#" + [registry] + token = "api-token" +"#; + +fn setup_old_credentials() { + let config = cargo_home().join("config"); + t!(fs::create_dir_all(config.parent().unwrap())); + t!(t!(File::create(&config)).write_all(&CONFIG_FILE.as_bytes())); +} + +fn setup_new_credentials() { + let config = cargo_home().join("credentials"); + t!(fs::create_dir_all(config.parent().unwrap())); + t!(t!(File::create(&config)).write_all(br#" + token = "api-token" + "#)); +} + +fn check_host_token(toml: toml::Value) -> bool { + match toml { + toml::Value::Table(table) => match table.get("token") { + Some(v) => match v { + &toml::Value::String(ref token) => (token.as_str() == TOKEN), + _ => false, + }, + None => false, + }, + _ => false, + } +} + +#[test] +fn login_with_old_credentials() { + setup_old_credentials(); + + assert_that(cargo_process().arg("login") + .arg("--host").arg(registry().to_string()).arg(TOKEN), + execs().with_status(0)); + + let config = cargo_home().join("config"); + assert_that(&config, existing_file()); + + let mut contents = String::new(); + File::open(&config).unwrap().read_to_string(&mut contents).unwrap(); + assert!(CONFIG_FILE == &contents); + + let credentials = cargo_home().join("credentials"); + assert_that(&credentials, existing_file()); + + contents.clear(); + File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap(); + assert!(check_host_token(contents.parse().unwrap())); +} + +#[test] +fn login_with_new_credentials() { + setup_new_credentials(); + + assert_that(cargo_process().arg("login") + .arg("--host").arg(registry().to_string()).arg(TOKEN), + execs().with_status(0)); + + let config = cargo_home().join("config"); + assert_that(&config, is_not(existing_file())); + + let credentials = cargo_home().join("credentials"); + assert_that(&credentials, existing_file()); + + let mut contents = String::new(); + File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap(); + assert!(check_host_token(contents.parse().unwrap())); +} + +#[test] +fn login_with_old_and_new_credentials() { + setup_new_credentials(); + login_with_old_credentials(); +} + +#[test] +fn login_without_credentials() { + assert_that(cargo_process().arg("login") + .arg("--host").arg(registry().to_string()).arg(TOKEN), + execs().with_status(0)); + + let config = cargo_home().join("config"); + assert_that(&config, is_not(existing_file())); + + let credentials = cargo_home().join("credentials"); + assert_that(&credentials, existing_file()); + + let mut contents = String::new(); + File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap(); + assert!(check_host_token(contents.parse().unwrap())); +} -- 2.30.2